Add ContributeAndFinalized replicate statuses#408
Conversation
| RESULT_UPLOADING, | ||
| RESULT_UPLOAD_FAILED, | ||
| RESULT_UPLOADED, | ||
| CONTRIBUTE_AND_FINALIZE_ONGOING, |
There was a problem hiding this comment.
Are we sure as REVEALING is not part of this list ?
There was a problem hiding this comment.
As far as I understand this part, this list should return all "non-uncompletable statuses", which is not the case right now. Also, it is used only for tests in iexec-core.
Let's rework the tests in iexec-core so that we can remove this method.
There was a problem hiding this comment.
Honestly, I would go for the consistency in a first step and to go for the cleanup in a second one.
There was a problem hiding this comment.
Are you sure about your "non-uncompletable" ?
For me it is the list of states allowing to go to COMPLETING state which are
REVEALED(worker part of the consensus)CONTRIBUTE_AND_FINALIZE_DONE(worker has revealed and has uploaded its results)RESULT_UPLOADsomething as the worker is part of the consensus and if this worker fails, to upload, another one could do it (to check if it is really supported)
There was a problem hiding this comment.
Please have a look at the only use of this method to understand what I mean:
https://github.com/iExecBlockchainComputing/iexec-core/blob/1e6d93d197b0346c4b46a62278d5b7286e56dd0f/src/test/java/com/iexec/core/replicate/ReplicateListenersTests.java#L164-L175
There was a problem hiding this comment.
After some discussions, we've decided to remove CONTRIBUTE_AND_FINALIZE_ONGOING from this list.
Please see b585b98.
| REVEALING, | ||
| REVEAL_FAILED, | ||
| CONTRIBUTE_AND_FINALIZE_ONGOING, | ||
| CONTRIBUTE_AND_FINALIZE_DONE, |
There was a problem hiding this comment.
I am not sure about keeping CONTRIBUTE_AND_FINALIZE_DONE here as the REVEALED state is not present in this list.
When CONTRIBUTE_AND_FINALIZE_DONE, result has been CONTRIBUTED and PROVED (aka REVEALED) on chain. I am not sure the task can be failed in this case.
I would rather comment this and ask a question to our experts when they are back.
There was a problem hiding this comment.
I do agree with your argument. Thanks for pointing it out!
Please see 258f8ae.
| RESULT_UPLOADING, | ||
| RESULT_UPLOAD_FAILED, | ||
| RESULT_UPLOADED, | ||
| CONTRIBUTE_AND_FINALIZE_ONGOING, |
There was a problem hiding this comment.
Honestly, I would go for the consistency in a first step and to go for the cleanup in a second one.
|
Kudos, SonarCloud Quality Gate passed! |
jbern0rd
left a comment
jbern0rd
left a comment
There was a problem hiding this comment.
Looks good to me !
LGTM !
2 participants
No description provided.